Skip to content

Conversation

kazutakahirata
Copy link
Contributor

We can express the fallback mechanism of llvm::countr_zero a lot more
concisely with llvm::popcount. Since llvm::countr_zero now requires
llvm::popcount, this patch moves llvm::popcount earlier.

We can express the fallback mechanism of llvm::countr_zero a lot more
concisely with llvm::popcount.  Since llvm::countr_zero now requires
llvm::popcount, this patch moves llvm::popcount earlier.
@llvmbot
Copy link
Member

llvmbot commented Sep 14, 2025

@llvm/pr-subscribers-llvm-adt

Author: Kazu Hirata (kazutakahirata)

Changes

We can express the fallback mechanism of llvm::countr_zero a lot more
concisely with llvm::popcount. Since llvm::countr_zero now requires
llvm::popcount, this patch moves llvm::popcount earlier.


Full diff: https://github.com/llvm/llvm-project/pull/158519.diff

1 Files Affected:

  • (modified) llvm/include/llvm/ADT/bit.h (+32-42)
diff --git a/llvm/include/llvm/ADT/bit.h b/llvm/include/llvm/ADT/bit.h
index d6e33c3e6133a..73aa84504d6e9 100644
--- a/llvm/include/llvm/ADT/bit.h
+++ b/llvm/include/llvm/ADT/bit.h
@@ -148,6 +148,35 @@ template <typename T, typename = std::enable_if_t<std::is_unsigned_v<T>>>
   return (Value != 0) && ((Value & (Value - 1)) == 0);
 }
 
+/// Count the number of set bits in a value.
+/// Ex. popcount(0xF000F000) = 8
+/// Returns 0 if the word is zero.
+template <typename T, typename = std::enable_if_t<std::is_unsigned_v<T>>>
+[[nodiscard]] inline int popcount(T Value) noexcept {
+  if constexpr (sizeof(T) <= 4) {
+#if defined(__GNUC__)
+    return (int)__builtin_popcount(Value);
+#else
+    uint32_t v = Value;
+    v = v - ((v >> 1) & 0x55555555);
+    v = (v & 0x33333333) + ((v >> 2) & 0x33333333);
+    return int(((v + (v >> 4) & 0xF0F0F0F) * 0x1010101) >> 24);
+#endif
+  } else if constexpr (sizeof(T) <= 8) {
+#if defined(__GNUC__)
+    return (int)__builtin_popcountll(Value);
+#else
+    uint64_t v = Value;
+    v = v - ((v >> 1) & 0x5555555555555555ULL);
+    v = (v & 0x3333333333333333ULL) + ((v >> 2) & 0x3333333333333333ULL);
+    v = (v + (v >> 4)) & 0x0F0F0F0F0F0F0F0FULL;
+    return int((uint64_t)(v * 0x0101010101010101ULL) >> 56);
+#endif
+  } else {
+    static_assert(sizeof(T) == 0, "T must be 8 bytes or less");
+  }
+}
+
 /// Count number of 0's from the least significant bit to the most
 ///   stopping at the first 1.
 ///
@@ -179,19 +208,9 @@ template <typename T> [[nodiscard]] int countr_zero(T Val) {
 #endif
   }
 
-  // Fall back to the bisection method.
-  unsigned ZeroBits = 0;
-  T Shift = std::numeric_limits<T>::digits >> 1;
-  T Mask = std::numeric_limits<T>::max() >> Shift;
-  while (Shift) {
-    if ((Val & Mask) == 0) {
-      Val >>= Shift;
-      ZeroBits |= Shift;
-    }
-    Shift >>= 1;
-    Mask >>= Shift;
-  }
-  return ZeroBits;
+  // Fallback to popcount.  "(Val & -Val) - 1" is a bitmask with all bits below
+  // the least significant 1 set.
+  return llvm::popcount(static_cast<std::make_unsigned_t<T>>((Val & -Val) - 1));
 }
 
 /// Count number of 0's from the most significant bit to the least
@@ -300,35 +319,6 @@ template <typename T> [[nodiscard]] T bit_ceil(T Value) {
   return T(1) << llvm::bit_width<T>(Value - 1u);
 }
 
-/// Count the number of set bits in a value.
-/// Ex. popcount(0xF000F000) = 8
-/// Returns 0 if the word is zero.
-template <typename T, typename = std::enable_if_t<std::is_unsigned_v<T>>>
-[[nodiscard]] inline int popcount(T Value) noexcept {
-  if constexpr (sizeof(T) <= 4) {
-#if defined(__GNUC__)
-    return (int)__builtin_popcount(Value);
-#else
-    uint32_t v = Value;
-    v = v - ((v >> 1) & 0x55555555);
-    v = (v & 0x33333333) + ((v >> 2) & 0x33333333);
-    return int(((v + (v >> 4) & 0xF0F0F0F) * 0x1010101) >> 24);
-#endif
-  } else if constexpr (sizeof(T) <= 8) {
-#if defined(__GNUC__)
-    return (int)__builtin_popcountll(Value);
-#else
-    uint64_t v = Value;
-    v = v - ((v >> 1) & 0x5555555555555555ULL);
-    v = (v & 0x3333333333333333ULL) + ((v >> 2) & 0x3333333333333333ULL);
-    v = (v + (v >> 4)) & 0x0F0F0F0F0F0F0F0FULL;
-    return int((uint64_t)(v * 0x0101010101010101ULL) >> 56);
-#endif
-  } else {
-    static_assert(sizeof(T) == 0, "T must be 8 bytes or less");
-  }
-}
-
 // Forward-declare rotr so that rotl can use it.
 template <typename T, typename = std::enable_if_t<std::is_unsigned_v<T>>>
 [[nodiscard]] constexpr T rotr(T V, int R);

Copy link

github-actions bot commented Sep 15, 2025

✅ With the latest revision this PR passed the C/C++ code formatter.

@kazutakahirata kazutakahirata merged commit d692380 into llvm:main Sep 15, 2025
11 checks passed
@kazutakahirata kazutakahirata deleted the cleanup_20250914_Support_countr_zero_popcount branch September 15, 2025 06:42
@llvm-ci
Copy link
Collaborator

llvm-ci commented Sep 15, 2025

LLVM Buildbot has detected a new failure on builder lldb-x86_64-debian running on lldb-x86_64-debian while building llvm at step 6 "test".

Full details are available at: https://lab.llvm.org/buildbot/#/builders/162/builds/31095

Here is the relevant piece of the build log for the reference
Step 6 (test) failure: build (failure)
...
PASS: lldb-api :: lang/cpp/pointer_to_member_type_depending_on_parent_size/TestPointerToMemberTypeDependingOnParentSize.py (855 of 3191)
PASS: lldb-api :: python_api/section/TestSectionAPI.py (856 of 3191)
PASS: lldb-shell :: SymbolFile/NativePDB/find-functions.cpp (857 of 3191)
PASS: lldb-api :: lang/cpp/non-type-template-param/TestCppNonTypeTemplateParam.py (858 of 3191)
PASS: lldb-api :: functionalities/scripted_process_empty_memory_region/TestScriptedProcessEmptyMemoryRegion.py (859 of 3191)
PASS: lldb-api :: functionalities/gdb_remote_client/TestGDBRemotePlatformFile.py (860 of 3191)
PASS: lldb-api :: functionalities/gdb_remote_client/TestGDBServerNoTargetXMLRegisters.py (861 of 3191)
PASS: lldb-api :: functionalities/json/object-file/TestObjectFileJSON.py (862 of 3191)
PASS: lldb-api :: functionalities/gdb_remote_client/TestGDBServerTargetXML.py (863 of 3191)
PASS: lldb-unit :: Core/./LLDBCoreTests/65/118 (864 of 3191)
FAIL: lldb-api :: functionalities/postmortem/netbsd-core/TestNetBSDCore.py (865 of 3191)
******************** TEST 'lldb-api :: functionalities/postmortem/netbsd-core/TestNetBSDCore.py' FAILED ********************
Script:
--
/usr/bin/python3 /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/dotest.py -u CXXFLAGS -u CFLAGS --env LLVM_LIBS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./lib --env LLVM_INCLUDE_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/include --env LLVM_TOOLS_DIR=/home/worker/2.0.1/lldb-x86_64-debian/build/./bin --arch x86_64 --build-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex --lldb-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-lldb/lldb-api --clang-module-cache-dir /home/worker/2.0.1/lldb-x86_64-debian/build/lldb-test-build.noindex/module-cache-clang/lldb-api --executable /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/lldb --compiler /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/clang --dsymutil /home/worker/2.0.1/lldb-x86_64-debian/build/./bin/dsymutil --make /usr/bin/gmake --llvm-tools-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./bin --lldb-obj-root /home/worker/2.0.1/lldb-x86_64-debian/build/tools/lldb --lldb-libs-dir /home/worker/2.0.1/lldb-x86_64-debian/build/./lib --cmake-build-type Release -t /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/functionalities/postmortem/netbsd-core -p TestNetBSDCore.py
--
Exit Code: -6

Command Output (stdout):
--
lldb version 22.0.0git (https://github.com/llvm/llvm-project.git revision d69238013fa10c3027975874b283b08a25e90fd0)
  clang revision d69238013fa10c3027975874b283b08a25e90fd0
  llvm revision d69238013fa10c3027975874b283b08a25e90fd0
Skipping the following test categories: ['libc++', 'msvcstl', 'dsym', 'gmodules', 'debugserver', 'objc']

--
Command Output (stderr):
--
Change dir to: /home/worker/2.0.1/lldb-x86_64-debian/llvm-project/lldb/test/API/functionalities/postmortem/netbsd-core
runCmd: settings clear --all

output: 

runCmd: settings set symbols.enable-external-lookup false

output: 

runCmd: settings set target.inherit-tcc true

output: 

runCmd: settings set target.disable-aslr false

output: 

runCmd: settings set target.detach-on-error false

output: 


/// Returns 0 if Value is zero.
template <typename T>
[[nodiscard]] inline int popcount(T Value) noexcept {
static_assert(std::is_unsigned_v<T>, "T must be an unsigned integer type");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My suggestion was that we allow unsigned types and call make_unsigned as a part of the implementation. Would there be any issues with that?

itzexpoexpo pushed a commit to itzexpoexpo/llvm-project that referenced this pull request Sep 21, 2025
We can express the fallback mechanism of llvm::countr_zero a lot more
concisely with llvm::popcount.  Since llvm::countr_zero now requires
llvm::popcount, this patch moves llvm::popcount earlier.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants